-
Notifications
You must be signed in to change notification settings - Fork 3.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
libbpf-tools/profile: Add support for PID-namespacing #5152
base: master
Are you sure you want to change the base?
Conversation
4f415e1
to
b0a1119
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have few comments, but it is looking really good!
libbpf-tools/profile.c
Outdated
@@ -595,6 +610,8 @@ int main(int argc, char **argv) | |||
env.perf_max_stack_depth * sizeof(unsigned long)); | |||
bpf_map__set_max_entries(obj->maps.stackmap, env.stack_storage_size); | |||
|
|||
set_pidns(obj); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can catch the returned value and show a warning message in case PID namespace does not exist?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed. thank you.
libbpf-tools/profile.c
Outdated
if (kernel_at_least(5, 7)) | ||
return -EPERM; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem with fixed version check is that they will not work on kernel where a specific patch could have been backported.
Is there a way to rather check for the existence of a given bpf helper?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In bcc/tools, we have a few cases (including profile) checking kernel version for bpf_get_ns_current_pid_tgid() availability. But for libbpf-tools, I think we can do better.
See trace_helpers.c. There are a few probe_*() functions which tries to detect whether a particular feature (prog_type, map_type) is supported or not. I think we can have another probe function to check whether bpf_get_ns_current_pid_tgid() is supported or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for good feedback.
I have applied the opinion.
@eiffel-fl
Could you please check if it's what you want?
Thank you.
libbpf-tools/profile.c
Outdated
@@ -595,6 +631,10 @@ int main(int argc, char **argv) | |||
env.perf_max_stack_depth * sizeof(unsigned long)); | |||
bpf_map__set_max_entries(obj->maps.stackmap, env.stack_storage_size); | |||
|
|||
err = set_pidns(obj); | |||
if (err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let us do 'err && env.verbose' here. We want to print out the below information under verbose mode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's modified. thank you.
libbpf-tools/profile.c
Outdated
if (kernel_at_least(5, 7)) | ||
return -EPERM; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In bcc/tools, we have a few cases (including profile) checking kernel version for bpf_get_ns_current_pid_tgid() availability. But for libbpf-tools, I think we can do better.
See trace_helpers.c. There are a few probe_*() functions which tries to detect whether a particular feature (prog_type, map_type) is supported or not. I think we can have another probe function to check whether bpf_get_ns_current_pid_tgid() is supported or not.
Add PID translation for nested PID namespace environments, such as container.
…he kernel Support PID namespace mapping only in kernels where bpf_get_ns_current_pid_tgid is available. Add a utility function to check if bpf_get_ns_current_pid_tgid is available in the target kernel. Use this function in the profile to determine support for PID namespace mapping.
Support PID namespace translation as memleak.py does.